feat(breadcrumb)!: spectrum 2 migration by jawinn · Pull Request #3395 · adobe/spectrum-css

jawinn · 2024-11-13T17:33:55Z

Description

Breadcrumbs Spectrum 2 Migration

The breadcrumbs component is now updated to use the S2 specs and tokens. This includes updated spacing, heights, colors, font sizes, etc. This does not currently include the updated S2 icons.

There has been a major design change to the variant classes related to density and sizing, to align their terminology better with t-shirt sizes:

The "compact" variant no longer exists. The spectrum-Breadcrumbs--compact class and associated custom properties are removed. The default (medium) breadcrumbs are now sized similar to what was previously called compact.
There is a new "large" variant, which uses the spectrum-Breadcrumbs--sizeL class. This is sized similarly to what was previously the default.

The breadcrumb title can now be customized in the multiline variant using an additional element that uses the typography component's heading classes. This and its preferred sizing have been added to the documentation, a new Docs example, and a new Storybook control titleHeadingSize. Design notes for reference:

The title item can be customized in the multiline variant, but the preferred sizes are Heading (S), Heading (M), Heading (L) (default), and Heading (XL)

The component has been refactored to slim down and simplify its CSS and custom properties, by changing the values of existing custom properties for large and multiline variants.

The drag and drop class has been better documented, and its class now has a background color using drop zone tokens for now per design feedback: "Those two use cases are essentially one in the same, as the breadcrumb item becomes a "drop zone" for that interaction."

The following mod custom properties have been removed:

--mod-breadcrumbs-action-button-spacing-block-between-multiline
--mod-breadcrumbs-action-button-spacing-block-compact
--mod-breadcrumbs-action-button-spacing-block-multiline
--mod-breadcrumbs-block-size-compact
--mod-breadcrumbs-block-size-multiline
--mod-breadcrumbs-font-family-compact
--mod-breadcrumbs-font-family-compact-current
--mod-breadcrumbs-font-family-multiline
--mod-breadcrumbs-font-family-multiline-current
--mod-breadcrumbs-font-size-compact
--mod-breadcrumbs-font-size-compact-current
--mod-breadcrumbs-font-size-multiline
--mod-breadcrumbs-font-size-multiline-current
--mod-breadcrumbs-font-weight-compact
--mod-breadcrumbs-font-weight-compact-current
--mod-breadcrumbs-font-weight-multiline
--mod-breadcrumbs-font-weight-multiline-current
--mod-breadcrumbs-icon-spacing-block-between-multiline
--mod-breadcrumbs-icon-spacing-block-compact
--mod-breadcrumbs-icon-spacing-block-start-multiline
--mod-breadcrumbs-text-spacing-block-between-multiline
--mod-breadcrumbs-text-spacing-block-end-compact
--mod-breadcrumbs-text-spacing-block-end-multiline
--mod-breadcrumbs-text-spacing-block-start-compact
--mod-breadcrumbs-text-spacing-block-start-multiline

The following mod custom properties have been renamed:

--spectrum-breadcrumbs-action-button-spacing-inline-start is now --spectrum-breadcrumbs-inline-start-to-truncated-menu to help clarify what it is used for. The general action button inline spacing is already handled by --mod-breadcrumbs-action-button-spacing-inline.

CSS-811

How and where has this been tested?

Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.

Validation steps

Breadcrumbs compared against S2 spec
New t-shirt size "large" exists and "compact" variant no longer exists
Small , medium, and multiline tested
New control titleHeadingSize works as expected and documentation around this is clear
New controls work as expected for truncated menu, root context, nav root text, and items (items was previously hidden)
Updated testing preview covers all variants, including added hover and focus-visible

Regression testing

Validate:

The documentation pages for at least two other components are still loading, including:

The pages render correctly, are accessible, and are responsive.

If components have been modified, VRTs have been run on this branch:

VRTs have been run and looked at.
Any VRT changes have been accepted (by reviewer and/or PR author), or there are no changes.

To-do list

I have read the contribution guidelines.
I have updated relevant storybook stories and templates.
I have tested these changes in Windows High Contrast mode.
If my change impacts other components, I have tested to make sure they don't break.
If my change impacts documentation, I have updated the documentation accordingly.
✨ This pull request is ready to merge. ✨

changeset-bot · 2024-11-13T17:34:01Z

🦋 Changeset detected

Latest commit: 0fe2a32

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@spectrum-css/breadcrumb	Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2024-11-13T17:41:31Z

File metrics

Summary

Total size: 2.22 MB^*
Total change (Δ): 🟢 ⬇ 5.23 KB (-0.23%)

Table reports on changes to a package's main file. Other changes can be found in the collapsed Details section below.

Package	Size	Δ
breadcrumb	13.79 KB	🟢 ⬇ 2.86 KB

Details

breadcrumb

Filename	Head	Compared to base
index.css	13.79 KB	🟢 ⬇ 2.86 KB (-17.17%)
metadata.json	7.50 KB	🟢 ⬇ 2.37 KB (-24.05%)

* Size determined by adding together the size of the main file for all packages in the library.
* Results are not gzipped or minified.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

github-actions · 2024-11-13T17:41:40Z

🚀 Deployed on https://pr-3395--spectrum-css.netlify.app

jawinn · 2024-11-14T20:01:49Z

I am seeing one issue where the text is rendering a little too high and those elements are greater than 32px tall (33.19px). This is related to line-height and the top and bottom padding token values, which is very similar to the discussion raised a few months back about button and line-height tokens. I'll bring this up with design again in Slack:

rise-erpelding

This looks really good! I only have a few small suggestions within the comments.

Also wondering: are the medium and large for breadcrumbs intended to be variants or t-shirt sizes? Is there some specific guidance somewhere that states that they can't or shouldn't be t-shirt sizes? I think the token specs make them seem more like variants, but the S2/Desktop spec makes them feel a bit more t-shirt size-like. It seems like, especially since --medium and --large are also used for specifying device size, it would be easier and less confusing to name them with the same conventions for t-shirt sizing, and that a future version of the component might follow t-shirt sizing even if this one doesn't. Thoughts?

components/breadcrumb/index.css

components/breadcrumb/stories/breadcrumb.stories.js

jawinn · 2024-11-14T19:57:01Z

components/breadcrumb/index.css

+	--spectrum-breadcrumbs-separator-spacing-inline: var(--spectrum-breadcrumbs-text-to-separator-multiline);
+	--spectrum-breadcrumbs-text-spacing-block-start: var(--spectrum-breadcrumbs-top-to-text-multiline);
+	--spectrum-breadcrumbs-text-spacing-block-end: var(--spectrum-breadcrumbs-top-text-to-bottom-text);
+	--spectrum-breadcrumbs-icon-spacing-block: var(--spectrum-breadcrumbs-top-to-separator-multiline) var(--spectrum-breadcrumbs-separator-to-bottom-text-multiline);


Do you think it's worth splitting --spectrum-breadcrumbs-icon-spacing-block into --spectrum-breadcrumbs-icon-spacing-block-start and --spectrum-breadcrumbs-icon-spacing-block-end versus setting them both in one like this? Same question for --spectrum-breadcrumbs-action-button-spacing-block.

components/breadcrumb/index.css

components/breadcrumb/stories/breadcrumb.stories.js

jawinn · 2024-12-20T22:02:16Z

Also wondering: are the medium and large for breadcrumbs intended to be variants or t-shirt sizes? Is there some specific guidance somewhere that states that they can't or shouldn't be t-shirt sizes? I think the token specs make them seem more like variants, but the S2/Desktop spec makes them feel a bit more t-shirt size-like. It seems like, especially since --medium and --large are also used for specifying device size, it would be easier and less confusing to name them with the same conventions for t-shirt sizing, and that a future version of the component might follow t-shirt sizing even if this one doesn't. Thoughts?

I've gone ahead and moved things around so large is now --sizeL, and there are separate size and variant controls. I agree that this seems better given that these are trying to align more with t-shirt sizes (there is a whole specs section under "reason for token updates" that goes over it). The way it previously was a variant and is presented in the designs where it's either default(medium), large, or multiline had made it feel like a variant. But I think this was is more consistent and future-proof if additional sizes get added.

jawinn · 2024-12-20T22:08:28Z

There are two things that I'd like to get answers on before re-review:

The action button is no longer displaying correctly on the spectrum-two branch. After this branch was rebased, the border on breadcrumb's quiet action button started showing. We'll have to look into what happened there and get that fixed on spectrum-two.
There was no design for "dragged". I updated the docs around this option after I figured out what it's doing. I'll ask design for specs on this; the older guidelines were showing a background color change that I don't see.

marissahuysentruyt

This first pass was just TODOs now that spectrum-two got its mega-update (I know we're waiting on some fixes and questions before another deep review). Looks like we can probably refactor in a couple places to use Container and size.

components/breadcrumb/stories/template.js

components/breadcrumb/stories/breadcrumb.stories.js

jawinn · 2025-01-31T16:25:49Z

@rise-erpelding

Should the large nested variant display the medium size action button?

The args for action button sizing are now updated to work with the change in variants, so s = multiline, m = size medium, l = size large.

We have default and large containers in the tests, so do we still need the "Sizing" container at the bottom?

Yes, good idea. I removed that by adding withSizes: false to the test.

Multiline comes in 4 sizes, but it looks like we're only testing small and default/medium. Would we want to test large and extra-large as well?

This now has its own test. Comments here on Marissa'a similar suggestion.

@marissahuysentruyt

Looking at the Figma desktop file for breadcrumbs...there's no dragged variant for the items. Is that something we need to keep/is dragged still supported in S2? It very well might be, just wanted to ask.

That was already something I confirmed with design about and they provided guidance on the dragged background color (some notes in PR description). I pinged them again to ask about adding this to the spec, as this will help with reviewing and comparing.

Do you think we need a focus state story on the docs page? Some of the other stories have them on their docs pages, but you did add the focus-visible test, so it's not like we haven't thought about it.

I don't think we need to show this on the Docs page in most cases, since we're using a storybook-only test class for VRT testing purposes for some of these interactions like focus, hover, active, etc. That could be a little confusing for referencing the markup to see a class not included in the CSS and it is not totally accurate in all cases. I don't see most button-like and form components Docs showing this currently. I think Textfield is one of the exceptions, since the docs were necessary to avoid some historical confusion and also show the quiet/non-quiet difference.

rise-erpelding

This update addresses all the things I'd noticed before!

I did notice one very minor thing by accident, because I had previously been checking cjk on another PR and had the locale flipped to Japan already:

When I switch to a cjk language, the multiline test/story with all the different-sized headings, it looks like the font size for .spectrum-Heading--sizeX shrinks a bit. There's no font-size adjustment for .spectrum-Breadcrumbs-item for cjk though so this font size stays the same. That makes it so that if no heading size is specified, the default heading size no longer matches any of the size options, if that makes sense. Basically, every single one of the current items in this screen shot has a different font for cjk (which you can kind of see visually if you look closely), instead of the default heading size always keeping pace with the large. The default with no heading size specified is 28px while the large is 25px. (It's also rendering different font stacks.)

I'm unsure if this is something that we can/should fix or whether maybe cjk was left out of the design for this? So I'll leave it to your discretion if it's something that needs to be addressed here and won't let it block my approval!

jawinn · 2025-01-31T19:27:27Z

Re: the CJK comments, Breadcrumbs does not have CJK styles defined in the spec, while the Heading component does. I've created a followup issue CSS-1113 to track, and started a conversation with the design team.

Also, about the "breadcrumb" vs "breadcrumbs" component naming, I found the existing issue CSS-1080; that would definitely be separate work as that will affect a lot of things.

marissahuysentruyt

This is looking good to me- thanks for addressing all of my comments!

Just a quick (non-blocking) question- how did you know what version to bump the breadcrumbs tokens? or did that update happen in the rebase (that's sort of what I'm thinking)?

Migrates component to Spectrum 2. Includes renaming of variants and new large t-shirt size, changed token usage to match spec, additional storybook options, etc. docs(breadcrumb): new controls for nested and improved nested docs Adds controls so a user is able to display the nested truncated menu, root context, set disabled items, and item text. This also clears up and moves around some of the documentation about the truncated menu and root context. docs(breadcrumb): refactor tests Update VRTs tests file to use new refactored controls and updated s2 variants. More options can now live in the state data. docs(breadcrumb): add hover and focus-visible to tests

jawinn added S2 Spectrum 2 size-3 M ~18-30hrs; moderate effort or complexity, several work days needed. labels Nov 13, 2024

jawinn changed the title ~~feat(breadcrumb): spectrum 2 migration~~ feat(breadcrumb)!: spectrum 2 migration Nov 13, 2024

jawinn force-pushed the jawinn/css-811-s2-breadcrumbs branch from 88c8ac3 to a819f2b Compare November 13, 2024 17:38

jawinn marked this pull request as draft November 13, 2024 19:31

jawinn marked this pull request as ready for review November 14, 2024 20:14

jawinn added the ready-for-review label Nov 14, 2024

rise-erpelding self-requested a review November 18, 2024 15:40

rise-erpelding requested changes Nov 18, 2024

View reviewed changes

jawinn commented Dec 2, 2024

View reviewed changes

castastrophe force-pushed the spectrum-two branch 3 times, most recently from cdb180d to 27d01df Compare December 4, 2024 14:54

jawinn force-pushed the jawinn/css-811-s2-breadcrumbs branch from 74ba42e to 402c6c6 Compare December 20, 2024 21:54

castastrophe force-pushed the spectrum-two branch 6 times, most recently from fc916f1 to 9d7f088 Compare December 29, 2024 19:50

jawinn force-pushed the jawinn/css-811-s2-breadcrumbs branch 2 times, most recently from 0a56d2b to 5ec2c31 Compare January 2, 2025 19:55

marissahuysentruyt reviewed Jan 6, 2025

View reviewed changes

jawinn force-pushed the jawinn/css-811-s2-breadcrumbs branch from 5ec2c31 to 5dd5dcc Compare January 6, 2025 17:32

jawinn force-pushed the jawinn/css-811-s2-breadcrumbs branch from 50fbb69 to aae0a89 Compare January 31, 2025 16:30

jawinn requested review from marissahuysentruyt and rise-erpelding January 31, 2025 16:33

rise-erpelding approved these changes Jan 31, 2025

View reviewed changes

marissahuysentruyt approved these changes Jan 31, 2025

View reviewed changes

jawinn force-pushed the jawinn/css-811-s2-breadcrumbs branch from aae0a89 to 4a56be2 Compare February 3, 2025 14:54

jawinn force-pushed the jawinn/css-811-s2-breadcrumbs branch from 4a56be2 to 0fe2a32 Compare February 3, 2025 14:54

jawinn merged commit 77aa82c into spectrum-two Feb 3, 2025
12 checks passed

jawinn deleted the jawinn/css-811-s2-breadcrumbs branch February 3, 2025 15:02

castastrophe mentioned this pull request Apr 15, 2025

feat(tokens,card): update to @adobe/spectrum-tokens@13.3.0 #3669

Merged

12 tasks

This was referenced May 16, 2025

chore(swatch,menu,swatchgroup): fix reverted metadata assets #3752

Merged

chore: merge latest updates from main #3777

Merged

castastrophe mentioned this pull request Jul 25, 2025

ci: chromatic runs for S2 [CSS-1233] #4070

Merged

3 tasks

castastrophe mentioned this pull request Aug 6, 2025

fix: minor linting updates #4113

Merged

11 tasks

This was referenced Aug 27, 2025

chore(popover): wrapper placement in story #4173

Closed

feat(storybook): chromatic a11y testing #4180

Closed

chore: updating lint-staged file inheritance order #4193

Closed

castastrophe mentioned this pull request Oct 22, 2025

feat(badge): remove modifiers from the API [SWC-1264] #4283

Merged

14 tasks

Conversation

jawinn commented Nov 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Breadcrumbs Spectrum 2 Migration

How and where has this been tested?

Validation steps

Regression testing

To-do list

Uh oh!

changeset-bot bot commented Nov 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions bot commented Nov 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

File metrics

Summary

breadcrumb

Uh oh!

github-actions bot commented Nov 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jawinn commented Nov 14, 2024

Uh oh!

rise-erpelding left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jawinn Nov 14, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jawinn commented Dec 20, 2024

Uh oh!

jawinn commented Dec 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marissahuysentruyt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jawinn commented Jan 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rise-erpelding left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jawinn commented Jan 31, 2025

Uh oh!

marissahuysentruyt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jawinn commented Nov 13, 2024 •

edited

Loading

changeset-bot bot commented Nov 13, 2024 •

edited

Loading

github-actions bot commented Nov 13, 2024 •

edited

Loading

github-actions bot commented Nov 13, 2024 •

edited

Loading

jawinn commented Dec 20, 2024 •

edited

Loading

jawinn commented Jan 31, 2025 •

edited

Loading

rise-erpelding left a comment •

edited

Loading